feat: make Spock the unified framework host - #15
Merged
Conversation
WalkthroughSpock 0.5.0 adds project manifests, scaffolding and adoption, a combined framework host with client/backend lifecycle handling, immutable runtime generations, authenticated Uhura assets, expanded CLI workflows, and guarded cross-platform npm packaging. ChangesFramework implementation
Distribution and release
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SpockCLI
participant SpockHost
participant BackendGeneration
participant UhuraClient
participant Browser
User->>SpockCLI: spock dev
SpockCLI->>SpockHost: serve_project
SpockHost->>BackendGeneration: capture and activate backend
SpockHost->>UhuraClient: load, validate, and publish client
SpockHost->>Browser: serve combined HTTP routes
Browser->>SpockHost: request status, authority, or client route
SpockHost-->>Browser: response or SSE invalidation event
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (2)
crates/spock-host/src/project.rs (1)
427-451: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftRename this status field or change the payload.
client_source_fingerprintis a source snapshot identity, butActiveClientStatus::artifact_fingerprintexposes it under an artifact name. Either publish a real served-artifact fingerprint here or rename the protocol field tosource_fingerprintbefore consumers rely on it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/spock-host/src/project.rs` around lines 427 - 451, Align the status payload with the value produced by client_source_fingerprint: either replace it with the fingerprint of the served artifact, or rename ActiveClientStatus::artifact_fingerprint and its protocol consumers to source_fingerprint. Ensure the published field name and semantics consistently identify the client source snapshot.crates/spock-project/src/layout.rs (1)
126-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deduplicating
expect_directory/expect_file.Both helpers are identical except for the metadata predicate and label text. A single generic helper parameterized by an
is_dir/is_filecheck would remove the duplication.♻️ Example consolidation
-fn expect_directory(path: &Path, label: &str, diagnostics: &mut Diagnostics) { - match fs::metadata(path) { - Ok(metadata) if metadata.is_dir() => {} - Ok(_) => diagnostics.push( - Diagnostic::new( - DiagnosticCode::WrongEntryKind, - format!("{label} is not a directory"), - ) - .at_path(path), - ), - ... - } -} - -fn expect_file(path: &Path, label: &str, diagnostics: &mut Diagnostics) { - match fs::metadata(path) { - Ok(metadata) if metadata.is_file() => {} - Ok(_) => diagnostics.push( - Diagnostic::new( - DiagnosticCode::WrongEntryKind, - format!("{label} is not a regular file"), - ) - .at_path(path), - ), - ... - } -} +fn expect_entry_kind( + path: &Path, + label: &str, + expected_kind: &str, + matches: impl Fn(&fs::Metadata) -> bool, + diagnostics: &mut Diagnostics, +) { + match fs::metadata(path) { + Ok(metadata) if matches(&metadata) => {} + Ok(_) => diagnostics.push( + Diagnostic::new( + DiagnosticCode::WrongEntryKind, + format!("{label} is not {expected_kind}"), + ) + .at_path(path), + ), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => diagnostics.push( + Diagnostic::new(DiagnosticCode::MissingInput, format!("{label} does not exist")) + .at_path(path), + ), + Err(error) => diagnostics.push( + Diagnostic::new(DiagnosticCode::Io, format!("could not inspect {label}: {error}")) + .at_path(path), + ), + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/spock-project/src/layout.rs` around lines 126 - 178, Deduplicate the metadata validation logic shared by expect_directory and expect_file by introducing one helper parameterized with the expected entry-kind predicate and message. Preserve each function’s existing DiagnosticCode, labels, path attachment, and directory-versus-regular-file wording while having both wrappers reuse the common implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/spock-cli/src/write_plan.rs`:
- Around line 438-443: The public apply_write_plan API must not invoke the
race-safe inner flow without a prepared filesystem target. Update
apply_write_plan to require and pass a retained prepared lease/target into
apply_write_plan_inner, or remove/rename this pathname-only entry point so it no
longer presents itself as race-safe; preserve the existing ApplySummary and
ApplyError behavior.
In `@crates/spock-host/src/assets.rs`:
- Around line 280-298: The asset validation flow around the manifest
verification loop and its subsequent serving logic must authenticate the sidecar
manifest against a trusted executable-embedded identity. Add a trusted manifest
digest, signature, or expected build identity to the binary, verify the loaded
manifest before comparing file entries, and reject mismatches before assets are
served; retain the existing file path, size, and SHA-256 checks.
In `@crates/spock-host/src/backend_capture.rs`:
- Around line 482-504: Update the backend source-loading flow around
canonical_source and the corresponding secondary input path to avoid reopening
validated paths with fs::read. Open each input through a no-follow,
root-constrained handle, validate the opened object’s identity and regular-file
status against the configured root, then read bytes from that same handle so
renames or symlink swaps cannot bypass PathEscape.
In `@crates/spock-host/src/events.rs`:
- Around line 72-103: Introduce shared session-wide admission control for all
event streams: update ProjectEventHub::subscribe and ProjectEventStream::drop in
crates/spock-host/src/events.rs (lines 72-103) to acquire a permit before
registering subscribers and release it on drop; update the project-event
subscription handling in crates/spock-host/src/http.rs (lines 145-150) to reject
requests when permits are exhausted; update the proxied Uhura stream flow in
crates/spock-host/src/http.rs (lines 303-365) to acquire the same limit and
release the permit on every termination path.
In `@crates/spock-host/src/generation.rs`:
- Around line 363-379: Update reject_client to distinguish initial rejection
from rejection while retaining an active client: set client_freshness to the
cold-rejected state when active_client is absent, and preserve RejectedLastGood
when it is present. Keep the existing editor_freshness behavior aligned with the
same active_client check.
In `@crates/spock-host/src/http.rs`:
- Line 185: Update the merged host router construction to remove
CorsLayer::permissive() or replace it with an explicit trusted-origin allowlist.
Ensure the authority and framework routes are not accessible from arbitrary
browser origins while preserving the existing router merge behavior.
In `@crates/spock-host/src/named_state.rs`:
- Around line 171-185: Update resolved_database_entry and the named-state
ownership flow to prevent hard-linked SQLite files from receiving independent
ownership based only on their final names. After resolving the database entry,
inspect the existing file’s identity and reject files with multiple hard links,
or use that identity in the lock digest and WAL/SHM coordination so all aliases
share ownership. Preserve the current path-based behavior for non-hard-linked
databases.
In `@crates/spock-host/src/project.rs`:
- Around line 388-415: Extend the final stability check after client preparation
to re-capture the client input fingerprint and compare it with the fingerprint
used for the immutable snapshot. Use the existing client-layout/capture symbols
around capture_stable_client and return HostError::UnstableProject when the
client inputs changed, while preserving the topology and backend checks.
In `@crates/spock-host/src/server.rs`:
- Around line 374-435: Update the publication failure branch after
begin_client_attempt to transition the client out of Building by recording the
attempt as rejected/failed and publishing the corresponding session event.
Preserve the last-good client state, or roll back to it when installation
already changed the active client, before emitting ObserverError so subscribers
receive the final invalidation.
- Around line 180-184: Update the shutdown sequence around the observer join
handling so its result is captured instead of propagated immediately; always
call lifecycle.shutdown().await before propagating an observer failure, then
propagate the captured join result while preserving server_result? handling.
In `@crates/spock-project/src/plan.rs`:
- Around line 568-585: Update finish_plan’s write-destination validation to
reject file/descendant collisions such as `foo` and `foo/bar`, not only
duplicate normalized paths. While iterating the sorted writes and using the
existing portable path normalization, detect when either the current path is an
ancestor of an already-seen path or an already-seen path is an ancestor of the
current path, emit a PlanConflict diagnostic, and prevent the plan from passing
preflight.
- Around line 158-164: Update the inventory entry classification around
file_type.is_symlink() and file_type.is_dir() so FIFOs, sockets, devices, and
other non-regular filesystem entries are rejected or represented as explicitly
unsupported rather than classified as InventoryEntryKind::File. Preserve
symlink, directory, and regular-file handling, and ensure unsupported entries
cannot become backend candidates.
In `@npm/scripts/sidecar.mjs`:
- Around line 137-140: Reorder the validation in regularFiles so
rootStat.isSymbolicLink() is checked before rootStat.isDirectory(). Preserve the
existing symlink-specific failure message, while using the missing-directory
error only for non-symlink paths that are not directories.
---
Nitpick comments:
In `@crates/spock-host/src/project.rs`:
- Around line 427-451: Align the status payload with the value produced by
client_source_fingerprint: either replace it with the fingerprint of the served
artifact, or rename ActiveClientStatus::artifact_fingerprint and its protocol
consumers to source_fingerprint. Ensure the published field name and semantics
consistently identify the client source snapshot.
In `@crates/spock-project/src/layout.rs`:
- Around line 126-178: Deduplicate the metadata validation logic shared by
expect_directory and expect_file by introducing one helper parameterized with
the expected entry-kind predicate and message. Preserve each function’s existing
DiagnosticCode, labels, path attachment, and directory-versus-regular-file
wording while having both wrappers reuse the common implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2fa7a32b-2a13-4827-9eac-388fc0e46659
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
.github/workflows/ci.yml.github/workflows/npm.yml.gitignoreCHANGELOG.mdCargo.tomlREADME.mdcrates/spock-cli/Cargo.tomlcrates/spock-cli/src/lib.rscrates/spock-cli/src/main.rscrates/spock-cli/src/project_commands.rscrates/spock-cli/src/write_plan.rscrates/spock-cli/tests/cli.rscrates/spock-host/.gitignorecrates/spock-host/Cargo.tomlcrates/spock-host/src/assets.rscrates/spock-host/src/backend_capture.rscrates/spock-host/src/client.rscrates/spock-host/src/events.rscrates/spock-host/src/generation.rscrates/spock-host/src/http.rscrates/spock-host/src/lib.rscrates/spock-host/src/named_state.rscrates/spock-host/src/project.rscrates/spock-host/src/routing.rscrates/spock-host/src/server.rscrates/spock-project/.gitignorecrates/spock-project/Cargo.tomlcrates/spock-project/src/diagnostic.rscrates/spock-project/src/discovery.rscrates/spock-project/src/layout.rscrates/spock-project/src/lib.rscrates/spock-project/src/manifest.rscrates/spock-project/src/path.rscrates/spock-project/src/plan.rscrates/spock-project/src/starter.rscrates/spock-project/templates/minimal-client/app/home/page.examples.uhuracrates/spock-project/templates/minimal-client/app/home/page.uhuracrates/spock-project/templates/minimal-client/catalog/base.tomlcrates/spock-project/templates/minimal-client/fixtures/empty.tomlcrates/spock-project/templates/minimal-client/fixtures/scripts/empty.tomlcrates/spock-project/templates/minimal-client/uhura.tomlcrates/spock-project/tests/project_flow.rscrates/spock-runtime/src/engine.rscrates/spock-runtime/src/generation.rscrates/spock-runtime/src/http.rscrates/spock-runtime/src/lib.rscrates/spock-runtime/studio/README.mdcrates/spock-runtime/tests/generation.rsdocs/rfd/0015-studio.mddocs/rfd/0020-distribution.mddocs/rfd/0022-spock-framework.mddocs/rfd/0023-development-state-reload.mddocs/rfd/README.mdnpm/LICENSEnpm/README.mdnpm/THIRD_PARTY_NOTICES.mdnpm/bin/spock.jsnpm/package.jsonnpm/scripts/sidecar.mjsrust-toolchain.tomluhura
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spockcommand project-aware with requiredspock.toml, one required backend, and an optional Uhura clientspock new,spock init, project-widecheck, fixedstart, and watcheddevwhile preserving the existing file-oriented language commandsspock-projectandspock-hostboundaries for manifest/discovery/scaffolding and combined generation/listener/lifecycle ownershipPins the merged Uhura
maincommit from gridaco/uhura#9 (baa70ce) through the existing submodule boundary.Doctrine and reload contract
Operational composition does not merge language ownership:
spock.tomlcomposes roots and lifecycle; it does not absorbuhura.tomlor reinterpret either language.spock devis deliberately client-live/backend-pinned. Valid client saves publish last-known-good generations. Backend source, referenced seed assets, and topology changes are watched and reported asrestart_required, but never reopen, reseed, migrate, or replace the active database. RFD 0023 retains the unsolved development-world/migration decision, with exactly oneTODO(RFD-0023)marker in Rust.Release-review hardening
Verification
HEAD/Content-Length, SSE admission/disconnect reuse, and shared project/Editor/Play stream limitsspock@0.5.0npm dry run: all four native builds, guarded 21-file / 18.4 MB tarball, dry publish, and installed framework-route/asset verification on macOS, Linux, and Windows704dc53generated no actionable comments; all 13 original review threads are resolvedExplicitly deferred